fix: allow resume_message_count=0 to disable resume history display#235
Open
RyanLHicks wants to merge 2 commits intompfaffenberger:mainfrom
Open
fix: allow resume_message_count=0 to disable resume history display#235RyanLHicks wants to merge 2 commits intompfaffenberger:mainfrom
RyanLHicks wants to merge 2 commits intompfaffenberger:mainfrom
Conversation
📝 WalkthroughWalkthroughget_resume_message_count now allows 0 as the minimum (previously 1) while still capping at 100; display_resumed_history short-circuits and returns early when the resolved message count is <= 0 to avoid rendering resumed history. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/config.py`:
- Line 1137: display_resumed_history currently uses
displayable_history[-num_messages:] which treats num_messages==0 as [-0:] and
returns the full list; update display_resumed_history() to explicitly handle
num_messages <= 0 by returning an empty list (or skipping display) before
performing the slice. Reference the symbols display_resumed_history,
displayable_history, and num_messages (and the config setting
resume_message_count) so you locate the consumer logic and add an early check
like "if num_messages <= 0: return []" (or equivalent control flow) to ensure
zero suppresses history as intended.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
get_resume_message_count() enforced a minimum of 1, making it impossible to fully disable the resume history display.
Changed the lower bound from 1 to 0 so users can set resume_message_count=0` to suppress the display entirely, as requested in #213.
Summary by CodeRabbit